-
Notifications
You must be signed in to change notification settings - Fork 244
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upgrade Substrate #2424
Upgrade Substrate #2424
Conversation
…make things compile, there are some warnings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one nit
@@ -577,6 +577,8 @@ where | |||
pub select_chain: FullSelectChain, | |||
/// Network service. | |||
pub network_service: Arc<NetworkService<Block, <Block as BlockT>::Hash>>, | |||
/// Cross-domain gossip notification service. | |||
pub cdm_gossip_notification_service: Box<dyn NotificationService>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps xdm_gossip_notification_service
to keep consistency with other places
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function is called cdm_*
that returns this thing. I have already discussed this with Ved, we will rename a few remaining things from cdm
to xdm
in a separate PR. Also feel free to push a commit here with renaming.
This is a pretty big upgrade and as such I limited it to upgrade of Substrate and those dependencies without which it didn't compile. There are many other dependencies to be updated that I'll take care of in a separate PR.
The first commit upgrades Substrate and changes everything that is broken, so it is a bit larger, further PRs address some deprecations an minor cleanups.
The only deprecation that is not fixed yet is
GenericChainSpec::from_genesis
, which I now think I know how to fix, but I spent way too much time on it already and will get back to local WIP branch once this PR lands.Notable upstream changes in Substrate:
sc-block-builder
: RemoveBlockBuilderProvider
paritytech/polkadot-sdk#2099chain-spec
: getting ready for native-runtime-free world paritytech/polkadot-sdk#1256 deprecatedGenericChainSpec::from_genesis
and movedcode
fromframe-system
's genesis config to the chain spec. I'm not sure it is a particularly helpful change, but this required to switch from genesis config to chain spec for building genesis state for domain, which is confusing because both have the same method for building storage, so things compiled, but crashed in runtime.sc-chain-spec
: add support for custom host functions paritytech/polkadot-sdk#2190 allows host functions in chain spec, which is how WASM runtime can access them in order to build genesis state (genesis state is no longer built with native code, but rather with WASM).NativeElseWasmExecutor
is no longer used paritytech/polkadot-sdk#2521 made it possible to get rid ofNativeElseWasmExecutor
since native runtime is no longer used by the node. While it wasn't strictly required, I removedNativeElseWasmExecutor
usage in a separate commit. It is a bit nicer that service crates now both define and implement host functions, though I'm not 100% happy that runtimes are still using those somewhat implicitly.sc-network
paritytech/polkadot-sdk#1370 is an even bigger network change that affected all gossip implementations that we have a few of. What I like about it is that now config and notification service are created together and that allowed me to discover that we don't actually use all of the gossip implementations, we created config fordomain-client-subnet-gossip
, but there was no use for its service. As such this crate was removed in a separate PR.ChainSync
paritytech/polkadot-sdk#2467 introduced massive sync layer changes in Substrate. Eventually it should be possible for us to inject DSN sync more properly into Substrate architecture and it shouldn't be that far from today.Smaller things in Substrate:
VrfOutput
toVrfPreOutput
andVrfSignature.output
toVrfSignature.pre_output
Slot
type paritytech/polkadot-sdk#1595 will allow us to take advantage of better typing around slots without converting to integers first, but will be in a seaprate PRRuntime[Hold/Freeze]Reason
paritytech/polkadot-sdk#1900 adds more boilerplate to pallet-balances config that from my understanding we don't need to worry about too muchThere were some small changes in Frontier, but nothing really notable, feel free to check the diff between
subspace-v4
andsubspace-v5
in our fork.A few changes from our fork of Substrate were upstreamed, which is nice, but networking changes caused a bunch of conflicts for our patches, so I had to adapt synced state announcement. I also reconsidered approach for sync pausing that allows us to switch to DSN sync and instead of introducing new sync mode after massive changes in paritytech/polkadot-sdk#2467 that made porting of the previous solution painful and invasive and replaced with atomic boolean instead that strategically disables a single operation. Should have the same effect, and while potentially a bit fragile (they can change sync process to request stuff from more places that this boolean will not help with and everything will continue to compile), the change is limited and hopefully temporary.
Overall here are all our patches on top of upstream right now, one of the biggest and most invasive is paritytech/polkadot-sdk#1598 that upstream consistently ignores 😞
As you can see, there will be a few follow-up PRs with cleanup of dependencies and such, but this is the bulk of the work.
Going forward I think it will be interesting to consider Polkadot-SDK's releases so we don't need to patch Frontier, but some of our changes do unfortunately require modification of Frontier to compile at all, not just upstream changes.
I do encourage you to look through all upstream changes in Substrate and Frontier if you have time to do so to familiarize yourself with what is happening.
Code contributor checklist: